Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency from bc #491

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

cousteaulecommandant
Copy link
Contributor

bc is used in the Makefile to perform simple arithmetic. While this utility is likely to be installed, it makes more sense to just use the shell's native $((...)) feature.

davideschiavone and others added 3 commits February 2, 2025 21:05
* Fix verilator URL

* Add verilator note on GCC version

* Update and unify some more

* Fix typo

* Remove non-getting-started instruction

* Unify
@davidmallasen
Copy link
Member

Is this needed in any environment @cousteaulecommandant? Otherwise, I would prefer to err on the side of caution and not merge to avoid breaking any setups.

@cousteaulecommandant
Copy link
Contributor Author

cousteaulecommandant commented Feb 9, 2025

If I recall correctly, the setup I used was based on a minimal Ubuntu setup inside a docker container and it didn't come with bc by default so updating to a newer version of X-HEEP broke the flow.

This might have been a corner case and installing bc is easy, but overall, I don't think it makes sense to depend on an external application (bc) that may not be installed, when the standard shell already does that (and saves you one process) via the builtin $(( )) syntax. If there's already syntax for that, why not use it?

@davidmallasen davidmallasen changed the base branch from main to develop February 10, 2025 09:07
@davidmallasen
Copy link
Member

Could you rebase to the develop branch? We'll stage the change there and merge to main later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants